Add permanent invitations and export medication changes as well#315
Open
pauljohanneskraft wants to merge 4 commits into
Open
Add permanent invitations and export medication changes as well#315pauljohanneskraft wants to merge 4 commits into
pauljohanneskraft wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds permanent invitations that can be reused across multiple users with seeded data retention, implements conditional invitation cleanup during enrollment based on permanence, and retrofits medication-request exports to reconstruct lifecycle dates (created, last-updated, removed) from Firestore history tracking rather than relying solely on live documents. ChangesPermanent Reusable Invitations
Medication History Export
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for reusable (permanent) invitation codes and extends the patient data export to include medication lifecycle information derived from the global /history collection.
Changes:
- Add a
permanentflag to invitations and adjust enrollment finalization to keep permanent invitations (and their seeded subcollections) for reuse. - Enhance medication export to reconstruct lifecycle records (created/updated/removed) by merging live documents with
/history. - Update/extend emulator tests and fixtures to cover permanent invitation reuse and medication lifecycle export.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| functions/src/tests/resources/patientExport.zip | Updates the expected export fixture (Git LFS pointer change). |
| functions/src/services/user/databaseUserService.ts | Keeps invitation docs/subcollections when invitation.permanent is set. |
| functions/src/services/export/defaultExportService.ts | Adds lifecycle reconstruction for medication requests and exports created/updated/removed timestamps. |
| functions/src/functions/exportData.test.ts | Adds test coverage for medication lifecycle reconstruction in export. |
| functions/src/functions/enrollUser.test.ts | Adds tests ensuring permanent invitations are reusable and non-permanent invitations are deleted. |
| functions/src/functions/createInvitation.test.ts | Adds assertions/tests for permanent defaulting to false and being set to true when requested. |
| functions/models/src/types/invitation.ts | Extends Invitation model + converter to include permanent with a default of false. |
| functions/models/src/functions/createInvitation.ts | Extends create-invitation input schema to accept optional permanent. |
| .gitignore | Ignores *.local.* files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.gitignore (1)
5-5: .gitignore*.local.*is broad—confirm intent/scope
- There are currently no files in this repo matching
*.local.*(nor*.local.js/json/ts), so this mainly affects future files.- Current “local” occurrences are in localization-related filenames (e.g.,
...+localization.ts,localizedText.ts) and don’t match the".local."pattern anyway.- Please confirm that “local” here is meant for local environment/config overrides; if so, consider narrowing the rule to the specific extensions you want (and/or add a brief comment).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore at line 5, The .gitignore entry "*.local.*" is overly broad and may accidentally ignore localization files; replace it with a narrowed pattern (e.g., target specific config/file extensions like "*.local.env", "*.local.json" or "*.local.js") and add a brief comment above the rule explaining it's for local environment/config overrides; update the rule where shown ("*.local.*") to the chosen specific patterns so localization-related filenames (e.g., files containing "localization" or "localizedText") are not inadvertently excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@functions/src/services/export/defaultExportService.ts`:
- Around line 353-356: The code currently sets created = entries.at(0)?.date
which can mistakenly treat the first tracked history row as creation; change
this so created is only populated when that history row is explicitly an
insertion: inspect the earliest nonNullEntries element for an insertion flag
(e.g. entry.operation === 'insert' or entry.isInsert === true) and only then set
created to that entry.date; otherwise leave created undefined/null and continue
to use lastUpdated = nonNullEntries.at(-1)?.date; update the logic around
nonNullEntries and created to reflect this conditional check.
---
Nitpick comments:
In @.gitignore:
- Line 5: The .gitignore entry "*.local.*" is overly broad and may accidentally
ignore localization files; replace it with a narrowed pattern (e.g., target
specific config/file extensions like "*.local.env", "*.local.json" or
"*.local.js") and add a brief comment above the rule explaining it's for local
environment/config overrides; update the rule where shown ("*.local.*") to the
chosen specific patterns so localization-related filenames (e.g., files
containing "localization" or "localizedText") are not inadvertently excluded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22af8df3-e9b8-4048-bd05-849850356268
⛔ Files ignored due to path filters (1)
functions/src/tests/resources/patientExport.zipis excluded by!**/*.zip
📒 Files selected for processing (8)
.gitignorefunctions/models/src/functions/createInvitation.tsfunctions/models/src/types/invitation.tsfunctions/src/functions/createInvitation.test.tsfunctions/src/functions/enrollUser.test.tsfunctions/src/functions/exportData.test.tsfunctions/src/services/export/defaultExportService.tsfunctions/src/services/user/databaseUserService.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #315 +/- ##
==========================================
+ Coverage 82.77% 82.90% +0.14%
==========================================
Files 93 93
Lines 3836 3877 +41
Branches 976 987 +11
==========================================
+ Hits 3175 3214 +39
- Misses 654 656 +2
Partials 7 7
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
pauljohanneskraft
temporarily deployed
to
staging
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test.yml:
- Around line 21-26: The reuseaction job lacks explicit least-privilege
permissions and currently relies on default GITHUB_TOKEN permissions which may
be broader than necessary. Add a permissions block to the reuseaction job
specifying only the minimum required permissions needed for REUSE compliance
checking, following the same principle as the buildandtest job permissions
defined on line 83.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 62050013-f750-4adc-840a-ff682eff4754
📒 Files selected for processing (5)
.github/workflows/build-and-test.ymlfunctions/models/src/functions/createInvitation.tsfunctions/src/functions/createInvitation.test.tsfunctions/src/functions/createInvitation.tsfunctions/src/services/export/defaultExportService.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- functions/src/functions/createInvitation.test.ts
- functions/models/src/functions/createInvitation.ts
- functions/src/services/export/defaultExportService.ts
Comment on lines
21
to
+26
| reuseaction: | ||
| name: REUSE Compliance Check | ||
| uses: StanfordBDHG/.github/.github/workflows/reuse.yml@v2 | ||
| markdownlinkcheck: | ||
| name: Markdown Link Check | ||
| uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2.3 | ||
| # markdownlinkcheck: | ||
| # name: Markdown Link Check | ||
| # uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2.3 |
There was a problem hiding this comment.
Set explicit least-privilege permissions for reuseaction.
Line 83 permissions only scope buildandtest; reuseaction currently falls back to default GITHUB_TOKEN permissions, which can be broader than needed.
Suggested fix
reuseaction:
name: REUSE Compliance Check
+ permissions:
+ contents: read
uses: StanfordBDHG/.github/.github/workflows/reuse.yml@v2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| reuseaction: | |
| name: REUSE Compliance Check | |
| uses: StanfordBDHG/.github/.github/workflows/reuse.yml@v2 | |
| markdownlinkcheck: | |
| name: Markdown Link Check | |
| uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2.3 | |
| # markdownlinkcheck: | |
| # name: Markdown Link Check | |
| # uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2.3 | |
| reuseaction: | |
| name: REUSE Compliance Check | |
| permissions: | |
| contents: read | |
| uses: StanfordBDHG/.github/.github/workflows/reuse.yml@v2 | |
| # markdownlinkcheck: | |
| # name: Markdown Link Check | |
| # uses: StanfordBDHG/.github/.github/workflows/markdown-link-check.yml@v2.3 |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 21-26: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 23-23: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-and-test.yml around lines 21 - 26, The reuseaction
job lacks explicit least-privilege permissions and currently relies on default
GITHUB_TOKEN permissions which may be broader than necessary. Add a permissions
block to the reuseaction job specifying only the minimum required permissions
needed for REUSE compliance checking, following the same principle as the
buildandtest job permissions defined on line 83.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add permanent invitations and export medication changes as well
♻️ Current situation & Problem
Invitation codes may also be made permanent as to allow to not create an invitation for each patient explicitly. Also, the patient data export should also contain medication changes.
⚙️ Release Notes
📚 Documentation
Please ensure that you properly document any additions in conformance to Spezi Documentation Guide.
You can use this section to describe your solution, but we encourage contributors to document your reasoning and changes using in-line documentation.
✅ Testing
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:
Summary by CodeRabbit
New Features
Bug Fixes
Chores